[F] Sudden changes in headphone volume #9
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
审阅者指南 by Sourcery这个拉取请求解决了在进入音乐选择时突然改变耳机音量的问题。它在 耳机音量调整过程的序列图sequenceDiagram
participant U as User
participant MS as MusicSelectProcess
participant SM as SoundManager
participant UD as UserDataManager
U->>MS: Enter music selection
MS->>UD: Get user data
UD-->>MS: Return user settings
Note over MS: Initialize volume fade-in
loop Every frame for 90 frames
MS->>MS: Update volume value
MS->>SM: Set headphone volume
end
Note over MS: Volume reaches target
音乐选择入口变更的类图classDiagram
class EntryToMusicSelection {
-int _timer
-CommonValue _volumeFadeIn
-bool[] _volumeFadeInState
+OnUpdate(InformationProcess)
+MapResultMonitorPreInitialize(int)
+MusicSelectProcessOnUpdate()
}
class CommonValue {
+float start
+float current
+float end
+float diff
+UpdateValue()
}
EntryToMusicSelection --> CommonValue
note for CommonValue "用于平滑音量过渡"
音量过渡状态图stateDiagram-v2
[*] --> InitialVolume
InitialVolume --> FadingIn: Start fade-in
FadingIn --> TargetVolume: After 90 frames
state FadingIn {
[*] --> Updating
Updating --> Updating: Update volume every frame
}
TargetVolume --> [*]
文件级变更
提示和命令与 Sourcery 交互
自定义您的体验访问您的仪表板以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request addresses an issue with sudden headphone volume changes when entering music selection. It implements a gradual volume increase over 90 frames within the Sequence diagram for headphone volume adjustment processsequenceDiagram
participant U as User
participant MS as MusicSelectProcess
participant SM as SoundManager
participant UD as UserDataManager
U->>MS: Enter music selection
MS->>UD: Get user data
UD-->>MS: Return user settings
Note over MS: Initialize volume fade-in
loop Every frame for 90 frames
MS->>MS: Update volume value
MS->>SM: Set headphone volume
end
Note over MS: Volume reaches target
Class diagram for EntryToMusicSelection changesclassDiagram
class EntryToMusicSelection {
-int _timer
-CommonValue _volumeFadeIn
-bool[] _volumeFadeInState
+OnUpdate(InformationProcess)
+MapResultMonitorPreInitialize(int)
+MusicSelectProcessOnUpdate()
}
class CommonValue {
+float start
+float current
+float end
+float diff
+UpdateValue()
}
EntryToMusicSelection --> CommonValue
note for CommonValue "Used for smooth volume transition"
State diagram for volume transitionstateDiagram-v2
[*] --> InitialVolume
InitialVolume --> FadingIn: Start fade-in
FadingIn --> TargetVolume: After 90 frames
state FadingIn {
[*] --> Updating
Updating --> Updating: Update volume every frame
}
TargetVolume --> [*]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @Becods - 我已经审查了你的更改 - 以下是一些反馈：
整体评论：
- 考虑将最小音量 (0.05f) 设置为可配置的常量，而不是硬编码的值，以提高可维护性和在不同硬件设置中的灵活性。
- 请在合并之前彻底测试2P（双人）场景，因为PR描述中提到尚未测试。
以下是我在审查期间查看的内容
- 🟡 一般性问题：发现2个问题
- 🟢 安全性：一切看起来都很好
- 🟢 测试：一切看起来都很好
- 🟢 复杂性：一切看起来都很好
- 🟢 文档：一切看起来都很好
帮助我变得更有用！请对每条评论点击 👍 或 👎，我将使用这些反馈来改进你的评论。
Original comment in English
Hey @Becods - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider making the minimum volume (0.05f) a configurable constant rather than a hardcoded value for better maintainability and flexibility across different hardware setups.
- Please test the 2P (two player) scenario thoroughly before merging, as indicated in the PR description that it hasn't been tested yet.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -14,6 +15,12 @@ namespace AquaMai.Mods.Tweaks.TimeSaving; | |||
| zh: "登录完成后直接进入选歌界面")] | |||
| public class EntryToMusicSelection | |||
| { | |||
| private static int _timer; | |||
|
|
|||
| private static CommonValue _volumeFadeIn = new(); | |||
There was a problem hiding this comment.
建议 (bug_risk)： 考虑使 _volumeFadeIn 特定于玩家，以避免竞争条件
当前的实现为两个玩家使用单个 CommonValue 实例，这可能导致两个玩家同时调整音量时出现意外行为。考虑使用数组：private static CommonValue[] _volumeFadeIn = new CommonValue[2];
建议的实现：
private static readonly CommonValue[] _volumeFadeIn = { new(), new() };这个更改还需要：
- 更新任何访问 _volumeFadeIn 的代码，使用适当的数组索引（例如，_volumeFadeIn[playerIndex] 而不是 _volumeFadeIn）
- 确保正确传递玩家索引到使用 _volumeFadeIn 的任何方法
Original comment in English
suggestion (bug_risk): Consider making _volumeFadeIn player-specific to avoid race conditions
The current implementation uses a single CommonValue instance for both players, which could lead to unexpected behavior when both players are adjusting volume simultaneously. Consider using an array: private static CommonValue[] _volumeFadeIn = new CommonValue[2];
Suggested implementation:
private static readonly CommonValue[] _volumeFadeIn = { new(), new() };This change will also require:
- Updating any code that accesses _volumeFadeIn to use the appropriate array index (e.g., _volumeFadeIn[playerIndex] instead of _volumeFadeIn)
- Ensuring the player index is properly passed to any methods that use _volumeFadeIn
| @@ -14,6 +15,12 @@ namespace AquaMai.Mods.Tweaks.TimeSaving; | |||
| zh: "登录完成后直接进入选歌界面")] | |||
| public class EntryToMusicSelection | |||
| { | |||
| private static int _timer; | |||
There was a problem hiding this comment.
建议 (bug_risk)： 计时器应该特定于玩家，以防止玩家之间的干扰
共享计时器可能导致一个玩家的淡入被另一个玩家的淡入中断。考虑使用数组：private static int[] _timer = new int[2];
建议的实现：
private static int[] _timer = new int[2];这个更改还需要：
- 更新使用
_timer的任何代码，指定要使用的是哪个玩家的计时器，例如_timer[playerIndex] - 确保正确初始化两个数组元素
- 更新任何计时器增加/减少操作，以处理特定玩家的计时器
由于我无法看到使用 _timer 的文件的其余部分，你需要在这些位置进行相应的更改。
Original comment in English
suggestion (bug_risk): Timer should be player-specific to prevent interference between players
A shared timer could cause one player's fade to be cut short if another player starts their fade. Consider using an array: private static int[] _timer = new int[2];
Suggested implementation:
private static int[] _timer = new int[2];This change will also require:
- Updating any code that uses
_timerto specify which player's timer to use, e.g.,_timer[playerIndex] - Ensuring proper initialization of both array elements
- Updating any timer increment/decrement operations to work with the specific player's timer
Since I can't see the rest of the file where _timer is used, you'll need to make these corresponding changes in those locations.
|
是不是没有在登出的时候重置 _volumeFadeInState |
Done |
clansty
reviewed
Jan 17, 2025
| @@ -95,4 +95,14 @@ public static void MusicSelectProcessOnUpdate() | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| [HarmonyPrefix] | |||
| [HarmonyPatch(typeof(ContinueProcess), "OnStart")] | |||
There was a problem hiding this comment.
万一没开续关不就没有 ContinueProcess
我觉得应该 hook 别的地方吧
不行的话到时候我改一下
我们是不是也可以在原先那个设置音量的地方 start coroutine |
|
改成了用协程实现
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
解决开启EntryToMusicSelection后音量突变问题
放在MusicSelectProcess防止调音量卡住ReleaseProcess影响体验
1P测试正常，2P情况下尚未测试，可能会存在bug
Summary by Sourcery
Bug 修复：
Original summary in English
Summary by Sourcery
Bug Fixes: